[wasm] Use ordinary struct alignment for Int128/UInt128 on wasm by lewing · Pull Request #131421 · dotnet/runtime

lewing · 2026-07-27T14:59:41Z

Int128/UInt128 were given a 16-byte alignment requirement on wasm by both the VM and crossgen2, annotated // sizeof(v128). Int128 is not a v128 — the only 128-bit value type in WebAssembly is v128, and WasmLowering.IsWasmV128Type recognizes only Vector128<T> and a 128-bit Vector<T>, so Int128 uses the generic by-reference struct ABI.

Every other alignment override in CheckForSystemTypes exists because the managed type corresponds to a fundamental data type in the target ABI (Vector128<T>↔__m128, and so on). Int128 has no counterpart on wasm, so it takes ordinary struct alignment — the 8 its two ulong fields produce. On the crossgen2 side that means routing wasm through the existing 32-bit path alongside ARM.

Symptom

The 16 was observable as wrong codegen at the interpreter→R2R boundary:

Int128 a = 1, b = 1;
a == b          // True
a.Equals(b)     // False   <-- R2R only
a.CompareTo(b)  // -1      <-- R2R only

Root cause

Wasm signature encoding spells a by-reference struct argument as S<N>, and RaiseSignature resolves S16 through GetCachedStructOfSize, which keys only on size and keeps the first struct seen at that size. The thunk therefore used that struct's 8-byte alignment while the interpreter used Int128's 16, placing the argument at argsBase+24 instead of argsBase+32. Equals and CompareTo read a shifted value; operator == takes its operands differently and was unaffected.

This also makes S<N> sound rather than accidentally correct. Single-field structs wrapping a v128 are unwrapped by LowerType and encode as V, and a v128 field is the only source of 16-byte alignment on wasm — so once Int128 takes its natural alignment, nothing reaching S<N> requires more than 8.

The VM and crossgen2 must change together or managed field layout desyncs.

Validation

Browser wasm, against builds with and without the change, rebuilding both corerun and the R2R composite for each variant:

	R2R off	R2R on
Int128 repro, before	correct	`Equals=False`, `CompareTo=±1` on equal values
Int128 repro, after	correct	correct

System.Text.Json, the Int128-bearing classes, 1496 tests, run on a branch carrying the other outstanding wasm R2R fixes so the suite completes:

	R2R off	R2R on
before	2 failed (0 Int128)	20 failed (15 Int128)
after	2 failed (0 Int128)	2 failed (0 Int128)

Those 2 fail with R2R off in both arms — pre-existing and unrelated.

Microsoft.Bcl.Memory 550/550 and System.Runtime.CompilerServices.Unsafe 128/128, R2R on and off.

Alternative considered

Encoding Int128 as V instead does not work. V is not an alignment tag — it selects a different calling convention, passing the value in a wasm v128 local rather than by reference. The JIT has no notion of Int128 at all, so R2R code faults with RuntimeError: function signature mismatch. Confirmed experimentally.

clang on wasm32 agrees with the by-reference treatment: it legalizes __int128 into two i64 parameters rather than any 128-bit entity, and passes an equivalent 16-byte struct as byval align 8. _Alignof(__int128) == 16 there is a memory-layout artifact, not a calling-convention requirement, since there is no 128-bit slot to align to.

wasm-only by construction — the VM change is inside #elif defined(TARGET_WASM) and the crossgen2 change adds TargetArchitecture.Wasm32 to an existing condition.

Note

This pull request was created with the assistance of GitHub Copilot.

Int128/UInt128 were given a 16-byte alignment requirement on wasm by both the VM and crossgen2, annotated "sizeof(v128)". Int128 is not a v128: the only 128-bit value type in WebAssembly is v128, and WasmLowering.IsWasmV128Type recognizes only Vector128<T> and a 128-bit Vector<T>, so Int128 uses the generic by-reference struct ABI. Every other alignment override in CheckForSystemTypes exists because the managed type corresponds to a fundamental data type in the target ABI (Vector128<T> to __m128, and so on). Int128 has no counterpart on wasm, so it takes ordinary struct alignment, which is the 8 its two ulong fields produce. On the crossgen2 side that means routing wasm through the existing 32-bit path. The 16 was observable as wrong codegen at the interpreter-to-R2R boundary. Wasm signature encoding spells a by-reference struct argument as S<N>, and RaiseSignature resolves S16 through GetCachedStructOfSize, which keys only on size and keeps the first struct seen at that size. The thunk therefore used that struct's 8-byte alignment while the interpreter used Int128's 16, placing the argument at argsBase+24 instead of argsBase+32. Int128.Equals and CompareTo then read a shifted value while operator== was unaffected. This also makes S<N> sound rather than accidentally correct: single-field structs wrapping a v128 are unwrapped by LowerType and encode as V, and a v128 field is the only source of 16-byte alignment on wasm, so once Int128 takes its natural alignment nothing reaching S<N> requires more than 8. The VM and crossgen2 must change together or managed field layout desyncs. Validated on browser wasm against builds with and without the change, rebuilding both corerun and the R2R composite for each: Int128 repro, R2R on: before, Equals=False and CompareTo=+/-1 on equal values; after, all correct. R2R off correct in both. System.Text.Json, the Int128-bearing classes, 1496 tests, on a branch carrying the other outstanding wasm R2R fixes: before, R2R-on 20 failed (15 Int128); after, 2 failed (0 Int128). Those 2 also fail with R2R off, so they are pre-existing and unrelated. Microsoft.Bcl.Memory 550/550 and Unsafe 128/128, R2R on and off. Encoding Int128 as V instead does not work: V selects a different calling convention, passing the value in a wasm v128 local rather than by reference, and the JIT has no notion of Int128, so R2R code faults with "function signature mismatch". Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ddfb2c-b6f8-4847-81e7-44d37d44a175

azure-pipelines · 2026-07-27T15:00:29Z

Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-07-27T15:01:15Z

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

lewing · 2026-07-27T15:05:02Z

I believe clang sets __int128 alignment to 16 and passes it as two 64 bit args so there is an argument to be made for special handling here but that change is larger and there is no current consumer.

lewing · 2026-07-27T15:06:03Z

cc @dotnet/wasm-contrib

Copilot

Pull request overview

This PR changes how System.Int128/System.UInt128 alignment is treated on WebAssembly so that wasm uses ordinary struct alignment (8) instead of a forced 16-byte alignment, keeping VM and crossgen2/R2R field layout consistent and avoiding ABI mismatches at interpreter↔R2R boundaries.

Changes:

In CoreCLR VM CheckForSystemTypes, switch wasm Int128/UInt128 alignment requirement from 16 to 8.
In crossgen2 field layout, treat TargetArchitecture.Wasm32 like existing 32-bit ARM behavior by using the standard metadata layout engine for Int128/UInt128.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/vm/methodtablebuilder.cpp	Updates wasm `Int128`/`UInt128` alignment requirement to 8 in system type layout overrides.
src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs	Routes wasm32 through the “32-bit metadata layout” path to match VM layout and avoid 16-byte override.

lewing · 2026-07-27T15:10:47Z

            // 32bit platforms use standard metadata layout engine
-            if (defType.Context.Target.Architecture == TargetArchitecture.ARM)
+            if (defType.Context.Target.Architecture is TargetArchitecture.ARM or TargetArchitecture.Wasm32)
            {


Correct that the comment is imprecise — X86 is 32-bit and does take the 16-byte override. That imprecision predates this change though, and the condition it describes is right there on the next line, so I'd rather not expand it here: rewording a pre-existing comment is unrelated churn, and per repo convention comments are kept minimal because inaccurate ones mislead and need maintenance.

Happy to fix it if a maintainer would prefer.

Note
This comment was generated by GitHub Copilot.

lewing · 2026-07-27T15:10:37Z

+            if (defType.Context.Target.Architecture is TargetArchitecture.ARM or TargetArchitecture.Wasm32)
            {
                layoutFromMetadata.LayoutAbiStable = true;
                layoutFromMetadata.IsInt128OrHasInt128Fields = true;


Added Wasm32 cases to TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence in 5fe365d. Both fail without the layout change (alignment 16 rather than 8), so they actually guard the new branch rather than just documenting it.

Note
This comment was generated by GitHub Copilot.

Covers the new Wasm32 branch in Int128FieldLayoutAlgorithm alongside the existing ARM/ARM64/X86/X64 cases. Both new cases fail without the layout change (alignment 16 rather than 8). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ddfb2c-b6f8-4847-81e7-44d37d44a175

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

+            // Wasm has no 128-bit scalar type to match, so use the natural field alignment.
+            pLayout->SetAlignmentRequirement(8);
 #else
 #error Unknown architecture
 #endif // TARGET_64BIT


tannergooding · 2026-07-27T15:35:57Z

I believe clang sets __int128 alignment to 16 and passes it as two 64 bit args so there is an argument to be made for special handling here but that change is larger and there is no current consumer.

This impacts packing and layout, something we really need to have correct from v1 as it can be breaking (behavioral, impact interop, invalidate R2R images, etc) to change it later.

The general tooling docs describing the basic C ABI (not required, just existing tooling convention) explicitly calls out 16: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#data-representation

emscripten then currently chooses 16-byte alignment/packing as well, in part because LLVM doesn't override it: https://github.com/llvm/llvm-project/blob/6baa9c9ac22c60e00471c6412387b5a48aa12e3e/clang/lib/Basic/Targets/WebAssembly.h#L28-L92 and https://github.com/llvm/llvm-project/blob/6baa9c9ac22c60e00471c6412387b5a48aa12e3e/llvm/lib/TargetParser/TargetDataLayout.cpp#L523-L533

Rust notably had a similar bug where they didn't use 16-byte alignment as expected and had to fix it last year: rust-lang/rust#133991

So I don't think we can take this and rather need to ensure the interpreter correctly handles this for stack data.

lewing · 2026-07-27T15:41:07Z

we have the same problem for v256/512, for which there are no intrinsic types either

tannergooding · 2026-07-27T15:43:55Z

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

lewing · 2026-07-27T15:45:24Z

You're right, and the BasicCABI table refutes my reasoning rather than just outweighing it. The row is:

General type	C Type	`sizeof`	Alignment	Wasm Value Type
Integer	`__int128_t`	16	16	(none)

My argument was "there is no corresponding wasm value type, therefore no ABI basis for the alignment override." The spec assigns alignment 16 to a type whose value type is explicitly (none), so that inference doesn't hold, and layout is a contract independently of how arguments are passed. Withdrawing this.

On the interpreter question: the interpreter looks like the side that's already correct. Tracing the original failure, it placed the Int128 argument at argsBase+32 (16-aligned); the R2R thunk placed it at argsBase+24. The thunk isn't consulting the type — LowerSignature spells a by-reference aggregate as S<N> with N = size only, and RaiseSignature turns S16 back into a type via GetCachedStructOfSize, which keys on size alone and keeps whichever 16-byte struct was lowered first.

Teaching S<N> about alignment would make the two agree, but it would still be the wrong calling convention: clang passes __int128 as two i64s by value, and a 16-byte struct as one i32 pointer.

callee    (long long, __int128,            int) -> (i32, i64, i64, i64, i32)
callee_s16(long long, struct{ulong,ulong}, int) -> (i32, i64, i32, i32)

That divergence is reachable rather than theoretical, since LowerSignature also drives IsUnmanagedCallersOnly. So Int128 likely needs its own signature encoding rather than reusing S<N> — closer to what V does for v128, though the wasm-level lowering would also need JIT support, as Int128 currently appears nowhere in src/coreclr/jit/. Still working that out.

Separately, the signature round-trip loses alignment for the vector types too, and that one is independent of whatever we decide for Int128. Reproduced on current main with no runtime changes, in a library crossgen'd for wasm where an 8-aligned 32-byte struct is lowered before the vector:

struct Blob32 { long a, b, c, d; }                       // 32 bytes, align 8  -> S32
static int Sum256(long tag, Vector256<int> v, int t)     // 32 bytes, align 32 -> S32
static int Sum512(long tag, Vector512<int> v, int t)     // 64 bytes, align 64 -> S64

R2R off:  Blob32 OK   Blob64 OK   Vector256 OK               Vector512 OK
R2R on:   Blob32 OK   Blob64 OK   Vector256 got=1002/1018    Vector512 got=1002/1026

Thunk signatures read out of the R2R image show the collision — Blob32 and Vector256<int> both emit S32:

WasmInterpreterToR2RThunk(llS32ip)   // Blob32
WasmInterpreterToR2RThunk(ilS32ip)   // Vector256<int>

Agreed no ecosystem contract is at stake for those yet. Worth noting the alignment isn't currently what you'd expect from 2x/4x V128 though: wasm has no branch in the Vector256/Vector512 handling in either CheckForSystemTypes or VectorFieldLayoutAlgorithm, so it falls to the final else and gets 32 and 64 rather than 16.

Note

This comment was generated by GitHub Copilot.

lewing · 2026-07-27T15:46:12Z

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

it requires another type signature I think, probably not huge but let me look.

lewing · 2026-07-27T15:56:26Z

we have the same problem for v256/512, for which there is no intrinsic type either

Those ones will require every ecosystem to take a break though, if WASM adds official types in the future. As of now, the layout/alignment should be correct as they should be recognized as 2 or 4x V128 fields

I think it's quite a bit different from Int128 which already has a defined contract today and where it will impact layout/interop with other targets.

Is getting the interpreter to handle the 16-byte stack alignment required here (and for Decimal128 which has its own PR up fixing layout on all targets, not just WASM), that much more complex?

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

tannergooding · 2026-07-27T16:07:44Z

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

They are defined as struct V256 { V128 lower, V128 upper } and struct V512 { V256 lower, V256 upper } and should be treated the same as equivalent user-defined structs (which is likewise how Arm64 treats them).

So a struct containing 2x V128 fields and a struct containing 2x structs, each containing 2x V128 fields. -- I expect WASM treats this the same as a struct containing 2x and 4x V128 fields, respectively. But some ABIs and our own layout algorithm/recursion should already be taking that into account when reporting things back.

lewing · 2026-07-27T16:18:15Z

My point was that they are also currently broken and have 32/64 alignment which probably doesn't make sense for the current wasm abi. So the question becomes how should we pass them? multiple v128s, or as a struct?

They are defined as struct V256 { V128 lower, V128 upper } and struct V512 { V256 lower, V256 upper } and should be treated the same as equivalent user-defined structs (which is likewise how Arm64 treats them).

So a struct containing 2x V128 fields and a struct containing 2x structs, each containing 2x V128 fields. -- I expect WASM treats this the same as a struct containing 2x and 4x V128 fields, respectively. But some ABIs and our own layout algorithm/recursion should already be taking that into account when reporting things back.

Right, I'm telling you the current handling is active broken because there is an alignment mismatch.

tannergooding · 2026-07-27T16:25:00Z

Right, I got that. I was explaining what the expected behavior is. If its incorrect today, then we probably need to fix it (I have no preference on whether that's the same or a separate PR)

lewing · 2026-07-27T16:59:57Z

Working on a separate prototype that can express the alignment and decomposition that matches the abi in the call signature without adding entirely new arg types.

Copilot AI review requested due to automatic review settings July 27, 2026 14:59

lewing requested a review from MichalStrehovsky as a code owner July 27, 2026 14:59

dotnet-policy-service Bot assigned lewing Jul 27, 2026

github-actions Bot added the area-VM-coreclr label Jul 27, 2026

Copilot started reviewing on behalf of lewing July 27, 2026 15:00 View session

lewing requested review from AndyAyersMS and davidwrighton July 27, 2026 15:05

lewing requested a review from jakobbotsch July 27, 2026 15:06

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 27, 2026 15:10

Copilot started reviewing on behalf of lewing July 27, 2026 15:11 View session

lewing requested a review from tannergooding July 27, 2026 15:18

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread src/coreclr/vm/methodtablebuilder.cpp

Comment on lines +10820 to 10824

// Wasm has no 128-bit scalar type to match, so use the natural field alignment.

pLayout->SetAlignmentRequirement(8);

#else

#error Unknown architecture

#endif // TARGET_64BIT

lewing marked this pull request as draft July 27, 2026 15:57

radekdoulik mentioned this pull request Jul 27, 2026

[browser][coreCLR] Track disabled OOP runtime tests pending browser support #131321

Open

41 tasks

radekdoulik mentioned this pull request Jul 27, 2026

Enable out-of-process tests for CoreCLR browser WASM #131110

Draft

lewing added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 27, 2026

lewing closed this Jul 27, 2026

Uh oh!

Conversation

lewing commented Jul 27, 2026

Symptom

Root cause

Validation

Alternative considered

Uh oh!

azure-pipelines Bot commented Jul 27, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

lewing Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

lewing Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tannergooding commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannergooding commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lewing commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lewing commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannergooding commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannergooding commented Jul 27, 2026

Uh oh!

lewing commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lewing commented Jul 27, 2026 •

edited

Loading

tannergooding commented Jul 27, 2026 •

edited

Loading

lewing commented Jul 27, 2026 •

edited

Loading

lewing commented Jul 27, 2026 •

edited

Loading

lewing commented Jul 27, 2026 •

edited

Loading